Skip to content

fix(aro-hcp): use shared helpers for in-place upgrades - #83113

Open
roivaz wants to merge 1 commit into
openshift:mainfrom
roivaz:fix-upgrade-inplace-exporter
Open

fix(aro-hcp): use shared helpers for in-place upgrades#83113
roivaz wants to merge 1 commit into
openshift:mainfrom
roivaz:fix-upgrade-inplace-exporter

Conversation

@roivaz

@roivaz roivaz commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Why

The in-place-upgrade rehearsal for #83084 exposed a pre-existing image override gap unrelated to the ARM-helper identity change.

The baseline at ARO-HCP commit 0767d80 resolved seven Region service images from ACR but omitted customExporter, leaving its static sha256:335a... digest next to commit-specific Backend and Fleet digests. The upgrade phase had the same omission because its override was assembled manually.

The complete workflow image audit found no other missing Region service image. hcpRecovery is intentionally absent from baseline ACR resolution because images-push does not publish it; the upgrade phase already supplies it. oc-mirror and tenant-quota are CI builds, not Region deployment overrides.

What

  • Replace the duplicated baseline implementation with the ARO-HCP hack/ci/provision-from-main.sh helper.
  • Build the upgrade override through hack/ci/build-config-override.sh.
  • Add the pipeline:aro-hcp-exporter dependency to the upgrade step.
  • Preserve the PR Hypershift operator and shared-ingress images through hypershift-image-overrides.yaml.
  • Leave the regular aro-hcp-provision-environment path unchanged.

Validation

  • Regenerated step-registry metadata.
  • Bash syntax and ShellCheck warning-level checks.
  • Baseline override test using the real ARO-HCP ACR resolver and override builder, confirming exporter, dual ARM helpers, and intentional absence of hcpRecovery.
  • Upgrade override test using the real ARO-HCP override builder, confirming exporter, hcpRecovery, Hypershift operator/shared-ingress, and dual ARM helpers.

Summary by CodeRabbit

  • Updates ARO-HCP in-place upgrade CI configuration to use shared provisioning and override-generation helpers.
  • Preserves PR Hypershift operator and shared-ingress images during upgrades.
  • Adds the pipeline:aro-hcp-exporter dependency and supplies the required customExporter override.
  • Keeps hcpRecovery out of baseline ACR resolution and supplies it only during upgrades.
  • Regenerates step-registry metadata and validates baseline and upgrade overrides, Bash syntax, and ShellCheck results.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

ARO-HCP provisioning now delegates to a shared helper. Local upgrade now requires EXPORTER_IMAGE, preserves selected Hypershift and ingress values, and delegates override generation. The pipeline reference supplies the exporter image and documents the shared helper.

Changes

ARO-HCP helper integration

Layer / File(s) Summary
Provisioning delegation
ci-operator/step-registry/aro-hcp/provision/from-main/aro-hcp-provision-from-main-commands.sh
The step replaces its inline provisioning workflow with hack/ci/provision-from-main.sh.
Upgrade override wiring
ci-operator/step-registry/aro-hcp/test/local-upgrade/aro-hcp-test-local-upgrade-commands.sh, ci-operator/step-registry/aro-hcp/test/local-upgrade/aro-hcp-test-local-upgrade-ref.yaml
The step requires EXPORTER_IMAGE, preserves Hypershift and shared ingress values, delegates override generation to build-config-override.sh, and wires the exporter pipeline dependency. Documentation describes the shared helper.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: avollmer-redhat, geoberle

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The patch changes only two shell scripts and one YAML file; it adds or modifies no Ginkgo test titles or dynamic test-name expressions.
Test Structure And Quality ✅ Passed Not applicable: the PR changes only Bash step scripts and YAML; the diff contains no Ginkgo test code or It/BeforeEach/Eventually/Expect blocks.
Microshift Test Compatibility ✅ Passed The patch changes only ARO-HCP shell scripts and YAML metadata; it adds no Ginkgo e2e tests or new OpenShift API usage requiring MicroShift checks.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The pull request changes only shell scripts and YAML; it adds no Ginkgo e2e tests, so SNO compatibility checks do not apply.
Topology-Aware Scheduling Compatibility ✅ Passed The patch changes only CI shell scripts and step metadata; it adds no deployment manifests, operators, controllers, replicas, affinity, topology spread, selectors, tolerations, or PDB constraints.
Ote Binary Stdout Contract ✅ Passed The PR changes only two shell scripts and one YAML file; it adds no Go/OTE binary or process-level suite code, so this stdout contract check is not applicable.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The commit changes only two shell scripts and one YAML reference; no new Ginkgo declarations, IPv4 literals, or public-network access appear in the changed files.
No-Weak-Crypto ✅ Passed The 3-file commit adds no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret/token comparisons; checks only file existence and config fields.
Container-Privileges ✅ Passed Changed ARO-HCP scripts and step reference add no privileged, host namespace, SYS_ADMIN, securityContext, or root execution settings.
No-Sensitive-Data-In-Logs ✅ Passed Changed scripts add no sensitive logging or tracing; they remove config dumps and digest/registry logs, while Azure login uses --output none and secret values are not passed to echo/printf.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: replacing duplicated ARO-HCP logic with shared helpers for in-place upgrades.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@roivaz

roivaz commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse pull-ci-Azure-ARO-HCP-main-e2e-parallel-inplace-upgrade

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 7, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@ci-operator/step-registry/aro-hcp/provision/from-main/aro-hcp-provision-from-main-commands.sh`:
- Line 17: Replace the failing delegation in aro-hcp-provision-from-main with
provisioning logic available in the checked-out tree, or add the required
hack/ci/provision-from-main.sh helper and invoke it reliably. Preserve the
downstream artifact contract by producing ${SHARED_DIR}/config.yaml and
${SHARED_DIR}/aro-hcp-slot.env.

In
`@ci-operator/step-registry/aro-hcp/test/local-upgrade/aro-hcp-test-local-upgrade-commands.sh`:
- Around line 77-86: Update the command flow around sourcing
hack/ci/build-config-override.sh so it uses the actual ARO-HCP upstream helper
available in openshift/release, adding or correcting the helper reference as
needed. Ensure the helper runs successfully before copying OVERRIDE_CONFIG_FILE
to config-override-upgrade.yaml and exporting the updated path.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 18c5abc7-ee5f-46a8-be8c-b0c934957232

📥 Commits

Reviewing files that changed from the base of the PR and between 88ce08f and 368676e.

📒 Files selected for processing (4)
  • ci-operator/step-registry/aro-hcp/provision/environment/aro-hcp-provision-environment-commands.sh
  • ci-operator/step-registry/aro-hcp/provision/from-main/aro-hcp-provision-from-main-commands.sh
  • ci-operator/step-registry/aro-hcp/test/local-upgrade/aro-hcp-test-local-upgrade-commands.sh
  • ci-operator/step-registry/aro-hcp/test/local-upgrade/aro-hcp-test-local-upgrade-ref.yaml

Use the shared ARO-HCP CI helpers for baseline provisioning and upgrade override generation so customExporter and both ARM helper identities stay aligned with ARO-HCP main.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@roivaz
roivaz force-pushed the fix-upgrade-inplace-exporter branch from 368676e to 70f27e8 Compare August 7, 2026 16:34
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@roivaz

roivaz commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse pull-ci-Azure-ARO-HCP-main-e2e-parallel-inplace-upgrade

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@roivaz: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel.

@openshift-ci

openshift-ci Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: roivaz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@roivaz: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-Azure-ARO-HCP-main-e2e-parallel-inplace-upgrade Azure/ARO-HCP presubmit Registry content changed
pull-ci-Azure-ARO-HCP-main-upgrade-e2e-parallel Azure/ARO-HCP presubmit Registry content changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@openshift-ci

openshift-ci Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@roivaz: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/rehearse/Azure/ARO-HCP/main/e2e-parallel-inplace-upgrade 70f27e8 link unknown /pj-rehearse pull-ci-Azure-ARO-HCP-main-e2e-parallel-inplace-upgrade

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@roivaz

roivaz commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

/pj-rehearse pull-ci-Azure-ARO-HCP-main-e2e-parallel-inplace-upgrade

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

@roivaz: your /pj-rehearse request was not processed because the request waited in queue for longer than 5 minutes. Please retry in a few minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant